Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

hurd: Avoid warnings #1268

Merged
merged 1 commit into from
Feb 6, 2024
Merged

Conversation

sthibaul
Copy link
Contributor

@sthibaul sthibaul commented Feb 4, 2024

The warning for line 37 was:

./pcap-hurd.c:196:35: warning: expression does not compute the number of elements in this array; element type is 'struct bpf_insn', not 'short' [-Wsizeof-array-div]
                          (filter_array_t_filter, FILTER_COUNT);
                                                  ^~~~~~~~~~~~

But device_set_filter really does want the size in units of shorts.

pcap-hurd.c Show resolved Hide resolved
@infrastation
Copy link
Member

How to get a functional Hurd VM:

  1. Install VirtualBox and qemu-utils.
  2. Download debian-hurd.img.tar.gz as shown on this page.
  3. Extract the .img file from the tarball and convert it to a VirtualBox disk image:
    qemu-img convert -f raw -O vdi debian-hurd-20210812.img debian-hurd-20210812.vdi
    
  4. Create a 32-bit VM and use the file as the disk.
  5. Boot and log in as root with no password.
  6. apt-get update; apt-get install git cmake flex bison autoconf clang libpcap-dev
  7. Create and use an unprivileged user.
  8. Clone the repositories and build as usual.

@infrastation
Copy link
Member

I also have a few fixes for tcpdump, will commit shortly.

@infrastation
Copy link
Member

Also, if the system has libdbus-1-dev installed (as a dependency of libpcap-dev, or otherwise), libpcap will not pass make check:

$ testprogs/findalldevstest
dbus-system
        Description: D-Bus system bus
        Flags:

dbus-session
        Description: D-Bus session bus
        Flags:

Error in pcap_lookupnet: SIOCGIFADDR: dbus-system: No such device

But that's a different bug.

@infrastation
Copy link
Member

Any objections to merge these changes?

@fxlb
Copy link
Member

fxlb commented Feb 4, 2024

Any objections to merge these changes?

It should be better to add the warning for L37 in the body on the commit message, like:

The warning for line 37 was:
<folded warning message>

@sthibaul
Copy link
Contributor Author

sthibaul commented Feb 4, 2024

Any objections to merge these changes?

It should be better to add the warning for L37 in the body on the commit message, like:

The warning for line 37 was:
<folded warning message>

done so

The warning for line 37 was:
(With clang version 11.0.1)
------------------------------------------------------------------------
./pcap-hurd.c:196:35: warning: expression does not compute the number of
elements in this array; element type is 'struct bpf_insn', not 'short'
[-Wsizeof-array-div]
                          (filter_array_t_filter, FILTER_COUNT);
                                                  ^~~~~~~~~~~~

./pcap-hurd.c:37:38: note: expanded from macro 'FILTER_COUNT'
#define FILTER_COUNT (sizeof(filter) / sizeof(short))
                             ~~~~~~  ^
./pcap-hurd.c:32:24: note: array 'filter' declared here
static struct bpf_insn filter[] = {
                       ^
./pcap-hurd.c:196:35: note: place parentheses around the 'sizeof(short)'
expression to silence this warning
                               (filter_array_t)filter, FILTER_COUNT);
                                                       ^
./pcap-hurd.c:37:38: note: expanded from macro 'FILTER_COUNT'
#define FILTER_COUNT (sizeof(filter) / sizeof(short))
                                     ^
------------------------------------------------------------------------

But device_set_filter really does want the size in units of shorts.
@fxlb
Copy link
Member

fxlb commented Feb 5, 2024

Commit message updated with full warning (place parentheses ...).

@fxlb
Copy link
Member

fxlb commented Feb 5, 2024

Any objections to merge these changes?

I have no more objections.

@infrastation infrastation merged commit a131e01 into the-tcpdump-group:master Feb 6, 2024
19 checks passed
@infrastation
Copy link
Member

Thank you.

@fxlb
Copy link
Member

fxlb commented Feb 7, 2024

But that's a different bug.

Should an issue be created on this subject?

@infrastation
Copy link
Member

#1269 it is.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants